Fix: serialize allowed_signature_methods correctly as JSON array#15
Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 32 minutes and 9 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mifiel/document.py (1)
43-70:⚠️ Potential issue | 🔴 CriticalFix the mixed
dict/tuple-list payload construction.Line 43 leaves
dataas adict, but lines 48 and 52 calldata.append(...);Document.create()will raiseAttributeErrorbefore sending the request. Keep scalar kwargs in a dict, collect repeated form fields separately, then convert to a list of tuples before callingprocess_request.Proposed fix
data = kwargs.copy() + viewers = data.pop('viewers', None) + form_data = [] for index, item in enumerate(signatories): for key, val in item.items(): if key == 'allowed_signature_methods' and isinstance(val, list): for method in val: - data.append( + form_data.append( (f'signatories[{index}][{key}][]', method) ) else: - data.append( + form_data.append( (f'signatories[{index}][{key}]', val) ) - if 'viewers' in data: - viewers = data.pop('viewers') + if viewers is not None: for index, item in enumerate(viewers): for key, val in item.items(): - data.update( - {'viewers[' + str(index) + '][' + str(key) + ']': val} + form_data.append( + (f'viewers[{index}][{key}]', val) ) if 'callback_url' in kwargs: data['callback_url'] = kwargs.get('callback_url') if file: mimetype = mimetypes.guess_type(file)[0] _file = open(file, 'rb') file = {'file': (basename(_file.name), _file, mimetype)} if dhash: data['original_hash'] = data.pop('dhash') + data = list(data.items()) + form_data doc = Document(client)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mifiel/document.py` around lines 43 - 70, The payload mixes a dict and list operations causing AttributeError; in the function building the request payload (in mifiel/document.py around the signatories/viewers handling used by Document.create()), keep scalar fields in a dict (e.g., data_dict) and accumulate repeated form fields (signatories' allowed_signature_methods and per-signatory entries, viewers entries) into a separate list of tuples (e.g., form_items); when done merge them by extending form_items with dict items converted to (key,value) tuples and use that list for process_request; also ensure file handling and dhash -> original_hash mapping write into the dict/form appropriately and that callback_url is set on the dict before final conversion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@mifiel/document.py`:
- Around line 43-70: The payload mixes a dict and list operations causing
AttributeError; in the function building the request payload (in
mifiel/document.py around the signatories/viewers handling used by
Document.create()), keep scalar fields in a dict (e.g., data_dict) and
accumulate repeated form fields (signatories' allowed_signature_methods and
per-signatory entries, viewers entries) into a separate list of tuples (e.g.,
form_items); when done merge them by extending form_items with dict items
converted to (key,value) tuples and use that list for process_request; also
ensure file handling and dhash -> original_hash mapping write into the dict/form
appropriately and that callback_url is set on the dict before final conversion.
This PR fixes an issue where the SDK was incorrectly sending `allowed_signature_methods` as a Python string instead of a JSON array, causing a 500 Internal Server Error when using mixed signature types like `FEA` and `FESCV`. ### What’s changed - In the `Document.create()` method, the `allowed_signature_methods` field is now properly serialized using `json.dumps()` before being sent in the form data. ### Why The Mifiel API expects `allowed_signature_methods` to be a JSON array (e.g. `["FEA"]`), not a Python string like `"['FEA']"` or `"FEA,FESCV"`. The previous version of the SDK unintentionally broke this compatibility when posting multipart/form-data requests.
…part/form-data Previously, allowed_signature_methods was being serialized either as a plain string or using indexed keys (e.g., [0], [1]), which resulted in incorrect array parsing by the Mifiel API backend. This commit updates the serialization to use the `[]` syntax: signatories[0][allowed_signature_methods][]=FEA signatories[0][allowed_signature_methods][]=FESCV This is a widely supported format for arrays in multipart/form-data and ensures compatibility with the backend’s expected structure. Also removes the use of json.dumps, which was turning arrays into escaped strings instead of sending them as proper arrays.
Replaced internal data structure with a list of tuples to correctly handle multiple allowed_signature_methods for each signer in multipart/form-data. This change ensures repeated keys are preserved, allowing the backend to receive arrays like: signatories[0][allowed_signature_methods][]=FEA signatories[0][allowed_signature_methods][]=FESCV instead of silently overwriting values when using a dict.
5d36fca to
ede92d8
Compare
Document.create mistakenly called dict.append when serializing signatories. Store scalar signatory fields on the dict, collect allowed_signature_methods as repeated bracket keys, then convert to a list of tuples for requests when needed. Apply callback_url and original_hash (dhash) before conversion so dict operations stay valid. Co-authored-by: Genaro Madrid <genaromadrid@users.noreply.github.com>
This PR fixes an issue where the SDK was incorrectly sending
allowed_signature_methodsas a Python string instead of a JSON array, causing a 500 Internal Server Error when using mixed signature types likeFEAandFESCV.What’s changed
Document.create()method, theallowed_signature_methodsfield is now properly serialized usingjson.dumps()before being sent in the form data.Why
The Mifiel API expects
allowed_signature_methodsto be a JSON array (e.g.["FEA"]), not a Python string like"['FEA']"or"FEA,FESCV". The previous version of the SDK unintentionally broke this compatibility when posting multipart/form-data requests.Summary by CodeRabbit